Wouldn't it be smarter to use standard filesystem library at this point instead of replacing platform independent code provided via 3p with platform specific code and API calls?

Agree, using std::filestystem where possible would be nicer. It should be mostly functional now that minimum MacOS version was bumped to 12. But personaly I would be happy with having less APR regardless (we have been planing to remove the thing for years, but it's just too entrenched).

Agree, using std::filestystem where possible would be nicer. It should be mostly functional now that minimum MacOS version was bumped to 12. But personally I would be happy with having less APR regardless (we have been planning to remove the thing for years, but it's just too entrenched).

With my last changes, use of the APR library is pretty much limited to the cache implementation and only very few other places. The cache is a somewhat daunting thing to replace with different functions. A solution that works 99% of the time is not enough for this. The rest seems pretty trivial to chuck once there is a working alternative, and very low risk.

Rather than waiting for std::filesystem to be fully functional it seems actually easier to extend LLFile with the necessary methods. I already have a halfway working implementation of a simple LLFile class instance with open(), seek(), tell(), and read() and write() methods. As long as the desired support for platforms is limited to Windows/Mac/Linux, it's not that a difficult exercise.
It even allows share locks, although on non-Windows it only provides advisory locks using the flock() function. Not perfect but enough to provide a means to let multiple viewer instances lock specific files from each other. Other applications with the necessary access rights but not using flock() still can clobber the files, but that is something that is almost impossible to prevent on non-Windows systems.

I did on purpose not create an all or nothing solution but wanted to do this step by step, starting with the more easy functions operating on just filenames.

But replacing the rename, remove, size and similar calls with std::filesystem methods may indeed be an option. I'll have a look at that. Still think that a fully RAII capable LLFile may be useful for normal file access. I'll check what std::filesystem can provide here and do some tests.

Agree, using std::filestystem where possible would be nicer. It should be mostly functional now that minimum MacOS version was bumped to 12. But personaly I would be happy with having less APR regardless (we have been planing to remove the thing for years, but it's just too entrenched).

There is one rather implicating point about using std::filesystem functions: They tend to throw exceptions on pretty much any underlaying error. The current viewer code is definitely not prepared to deal with that in most places, so replacing functions like LLFile::rename() or LLFile::remove with the according std::filesystem functions is going to be a fairly substantial refactoring exercise. Anyone willing to venture into that?

There is one rather implicating point about using std::filesystem functions: They tend to throw exceptions on pretty much any underlaying error.

They only do if you use the throwing version of the methods. Each method is overloaded and take a std::error_code parameter - those are non-throwing.

The current viewer code is definitely not prepared to deal with that in most places, so replacing functions like LLFile::rename() or LLFile::remove with the according std::filesystem functions is going to be a fairly substantial refactoring exercise. Anyone willing to venture into that?

The idea was to use std::filesystem inside LLFile, not replacing LLFile everywhere.

There is one rather implicating point about using std::filesystem functions: They tend to throw exceptions on pretty much any underlaying error.

They only do if you use the throwing version of the methods. Each method is overloaded and take a std::error_code parameter - those are non-throwing.

The current viewer code is definitely not prepared to deal with that in most places, so replacing functions like LLFile::rename() or LLFile::remove with the according std::filesystem functions is going to be a fairly substantial refactoring exercise. Anyone willing to venture into that?

The idea was to use std::filesystem inside LLFile, not replacing LLFile everywhere.

Thanks for the nudge into the right direction. I was initially seriously considering to throw out LLFile too and directly call std::filesystem and co, but that would be a real mess to manage.

So it appears that use of std::filesystem for the path related operations would be a viable option and I kind of like the noexcept variants of that interface. There are slight adaptions of behavior necessary mostly related to the viewers preferences to treat some errors not as an error at all and create_directory() does not have a permission mask, which is a minor inconvenience since the only place it is used is an obscure dir_exists_or die() and according to a comment inside, this function is only used in the simulator, which I don't think is any concern for the current viewer codebase. Even if it was, that call uses the current default value too, so not sure what relevance it has at all.

For a sane file read/write access to fully replace APR we would then have to use ifstream/ofstream and/or fstream to make LLFile a thin wrapper around this interface. The only three drawbacks I see with this:

1. There seems no public stat() like functionality in the STL
   The viewer uses this in several places to not only detect if a file or directory exists but to also then use the st_ctime, st_mtime, and st_size. The STL provides functions to return the two last values but it will do the whole string->path->openfile->stat->closefile each time. There seems no way to retrieve the st_ctime through this interface.

2. No support of long paths under Windows
   The STL maintainers have repeatedly refused to make the Windows STL implementation long path capable mainly on the grounds of backwards compatibility and their believe that they can not implement a fully safe path normalization, which would be mandatory since the long path name prefix bypasses most kernel interface path normalization and sanitation measures and passes the path unchecked to the kernel itself. This is relatively harmless in an environment where users can't directly enter full pathnames, but the STL library doesn't have the comfort of such a guarantee.

3. No shared/exclusive file locking on open
   This would be a very handy feature as it makes management of concurrent access to files from different processes a lot easier. The fact that it is only advisory on non-Windows systems isn't a killing argument, as it still would allow to manage concurrent access to files from multiple viewer instances. The native_handle that could make this possible to call the according platform function is unfortunately only a C++26 feature, so a nogo here.

And fstreams error handling is a bit awkward, not in any way similar to the more modern std::filestream error handling. Yes we could enable exceptions() on the fstream, but that does not make for cleaner code either and adds quite a bit of complexity in getting it exactly right.

So it appears that use of std::filesystem for the path related operations would be a viable option and I kind of like the noexcept variants of that interface.

Viewer already uses std's existance check in some places (not sure why), can act as an example.

and according to a comment inside, this function is only used in the simulator

You can safely ignore that. There is no mechanism to sync viewer/simulator base even for llsd, much less LLFile.

No shared/exclusive file locking on open

That's the bigger problem with replacing APR's file access (but APR as a whole has some additional issues with network and processes).

P.S. Also note that some places use boost::filesystem. Some of that is safe to replace with std::filesystem and boost can pad missing std support in other places (as not everything is supported on mac). boost::interprocess::file_lock might be a compensation for APR's locking.

So it appears that use of std::filesystem for the path related operations would be a viable option and I kind of like the noexcept variants of that interface.

Viewer already uses std's existance check in some places (not sure why), can act as an example.

I think it would be useful to make the whole viewer all use the same function, most probably the LLFile wrapped function.

No shared/exclusive file locking on open

That's the bigger problem with replacing APR's file access (but APR as a whole has some additional issues with network and processes).

The file_lock is currently only used in one place really: llappviewer.cpp for the marker file itself.
It might be possible to hack that sort of into. Accessing the native handle of an existing fstream is not an option as that is only a C++26 feature, but a possible workaround might be to create a file descriptor in the old way and wrap it in a stream_buf. The possibility seems to exist but it is not really part of the official standard. That way we would have access to the underlaying file descriptor and can do flock() on that (or LockFileEx() on Windows). But it would probably be an option in the open() call, not a separate method.

P.S. Also note that some places use boost::filesystem. Some of that is safe to replace with std::filesystem and boost can pad missing std support in other places. boost::interprocess::file_lock might be a compensation for APR's locking.

boost::filesystem is basically the origin of what eventually got std::filesystem but with adaptions. So does boost::filesystem try to support the long path name prefix, although I'm not sure it fully can handle it. It at least does not bork the path when you try to determine the root of a path that has the Windows NT kernel object space prefix. More functional IMHO would be to keep that completely out of the actual path layer and to simply apply that prefix whenever a path is approaching the MAX_PATH limit before passing it to the WinAPI wide char function. But that does require proper path sanitation and while I did some of that in the old implementation it is not really enough to sanitize random user entered paths.

I'm running into some rather annoying problems trying to run the unit tests.

The first problem are the annoying firewall popups that appear for every other test or so. There must be a solution for this!

Then the tests randomly seem to get stuck after reporting success. They typically start, do their thing report that they happily succeeded but never quite terminate. By far not all but enough to be a real nuisance. After 4 or so such stuck tests, the Visual Studio process scheduler has exhausted its pool and sits just idling and waiting for one of these to finally return. The only thing that helps is to go into Task Schedular and terminate the child process for these stuck tests, which of course causes the batch wrapper to report an error, but lets Visual Studio continue with launching more tests until it exhausts its pool again.

You should be able to run specific tests individually instead of all of them. Never had that issue with firewall or tests being stuck, don't have any ideas how to fix that.

You should be able to run specific tests individually instead of all of them. Never had that issue with firewall or tests being stuck, don't have any ideas how to fix that.

That's my plan but I found a few other tests that depend directly or indirectly on LLFile and need to run them too, to be sure. One specific change to remove rmdir() since remove() will happily do that now too, did actually break compiling for 3 or so tests. Rather than trying to remember what all could break in the myriad of tests and how to look for these occurrences it seemed easier to just build them and run them all, except it isn't ;-(

The Firewall dialog is maybe not the actual issue. I couldn't so far establish a relationship between the tests this dialog popups for and the processes that get stuck on exit. In fact there are tests that terminate cleanly and terminate properly where this dialog actually appears, long before I get a chance to acknowledge the dialog. But those dialogs for sure are very annoying and defeat any unit test run attempt as they require you to be present at the machine to acknowledge them.

And that process hang after the test has successfully ended, really irks me. But it reminds me vaguely on a similar issue when running external tools in Visual Studio through a batch file that could make them sometimes hang after process termination. Will try a command line run of the whole autobuild build command as next step.

I didn't scrutinize every line (tad bit too much volume), but superficially I see no issues, when I tried running it files got removed/renamed correctly and test should be able to cover changes fine (but I will add a test plan for QA either way).

Is this ready to merge?

Is this ready to merge?

Yes, as far as I'm concerned I think it can be merged. I did not find weird things so far and compiled and run it under MacOS too, so it should be ok to merge. Still planning to get a complete Ubuntu toolchain setup and ready too, but that is not at the top of my priorities right now.

I do have more changes that remove more of LLAPRFile in specific parts, but they can be treated as their own independent changes. Ultimately I have a version compiling and seemingly working fine, that works with LLAPRFile and its pools completely removed. But that involves parts such as specific import and export functions and BVHLoader etc. that are not tested in any way by simply running the Viewer. So will have to treat that carefully as I'm not able to upload content (both from a permission point of view as well as the technical ability).

Also have been looking at the possibility to add support for memory mapped files to LLFile. It seems not to complicated although there are potentially a few differences/limitations under Windows compared to the other platforms. Needs some more thought and some testing and experimenting. With memory mapping working, there could potentially be quite some performance gains possible when loading textures and such.

Thank you, merged!

Sadly this interferes with linux work, I'm reverting in develop and cherry picking into a different branch. Supposedly untill linux support releases, and here I hoped we will get some progress on getting rid of crashy apr. Perhaps I will try making a PR into a linux branch and will fix issues there.

For now it's here:
e209d1c

Sadly this interferes with linux work, I'm reverting in develop and cherry picking into a different branch. Supposedly untill linux support releases, and here I hoped we will get some progress on getting rid of crashy apr. Perhaps I will try making a PR into a linux branch and will fix issues there.

What is the expected timeframe for the Linux release? Is that in conflict with develop_linux?

Is that in conflict with develop_linux?

I assume that it is.
#4927

What is the expected timeframe for the Linux release?

Target was 'after 2025.08', which just released, how it's going to turn out nobody knowns as it's the end of the year and plans are foggy. My guess: it will get out as 2026.02 as another project depend on linux support and 2026.01 is already 'cut out'.

Well, with a PR changing 300+ files, it's almost impossible not to have any conflicts!

I see that both PRs affect llapp.cpp, llapr.cpp, llerror.cpp, llcrashlock.cpp (which goes away), lldir_win32.cpp, lldir_test.cpp, llshadermgr.cpp, llappviewer.cpp, llfloaterpreference.cpp, lltexturecache.cpp, llviewerdisplay.cpp, llviewermedia.cpp and test/CmakeLists.txt

From these llcrashlock.cpp, lltexturecache.cpp, llviewermedia.cpp and test/CmakeLists.txt have actual conflicts although lltexturecache.cpp is exactly the same removal of dead code.

The big question is of course, what to do with the other changes I already have in my local repository? It's all in all a similar sized changeset as this has already been! And for sure going to affect a few more of the linux changed files.

I haven't attempted an actual merge yet with that PR, but from above list that I created with a side by side view of the PR changes, the real conflicts seem very limited. Yes git can sometimes get confused when attempting to merge even if the changes do not overlap, but the only real conflicts are limited to llcrashlock.cpp, lltexturecache.cpp, llviewermedia.cpp and test/CmakeLists.txt and they all seem fairly trivial to resolve.

Considering that a 300+ file PR is VERY hard to test, verify, apply and QA, it is very questionable if it gets even into 2026.02. And there will be more conflicts arising in the months until then. Are we going to postpone them all until after #4927 was merged? From a very quick and cursory look at that PR there is quite a bit in there that is fairly isolated and could and should have been applied already as its own patch. That would make it also more easy to apply the remainder eventually.

@fmartian We're targeting 2026.02 at this time - a good bit of review work was done on that, and the main thing we're worried about is how it impacts other platforms (since there was a bit of GL init cleanup, some dusting off of GL headers, and so on). A lot of the work there is reasonably contained in the interests of making Linux work. This should go in after that. This is largely a risk mitigation effort given what we currently have on our plate for the next couple of releases. Sorry for the trouble here.

We're targeting 2026.02 at this time - a good bit of review work was done on that, and the main thing we're worried about is how it impacts other platforms (since there was a bit of GL init cleanup, some dusting off of GL headers, and so on). A lot of the work there is reasonably contained in the interests of making Linux work. This should go in after that. This is largely a risk mitigation effort given what we currently have on our plate for the next couple of releases. Sorry for the trouble here.

What I was trying to say is that it would likely take me one hour or less to resolve any resulting merge conflicts with this in #4927. And although I'm not quite sure how to go about it at the moment to append a patch to #4927 myself, I would actually be willing to learn that if it is an option.